-
Notifications
You must be signed in to change notification settings - Fork 36.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RPC: minetxlocally #21372
RPC: minetxlocally #21372
Conversation
Doing this is pretty bad for block propagation performance-- probably adding over 100ms to the global propagation time because it guarantees a round trip at every hop. ... especially now with fibre mostly gone. Last I looked the only pool mining there own non-standard transactions was f2pool (>100kb in their case). This facility would also make it extremely easy to unintentionally fork the network by mining transactions which use functionality which is reserved for future consensus rules which may already be enforced by part of the network. Particularly because it's not realistic for any external software to enforce softfork safeness, since you need a whole script interpreter to do so -- an obvious thing someone might try to do with this interface is make a "transaction accelerator" but in doing so they'd accidentally introduce a public facing network split generator. |
I don't get this part, are you referring to Compact Blocks? Mining pool nodes receive full blocks from each other with transactions, adding an unbroadcast transaction wouldn't break anything. I aimed to give the miner more control by this change (not every miner has a fork of Core) and now it seems that the best is to add a warning to the RPC description since the last thing a miner wants is a network fork. I don't think this feature can be abused, it's only for convenience. For an evil miner, this doesn't make anything easier. They can remove |
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
Compact blocks are able to relay blocks without any roundtrips if all the blocks' transactions (excluding the coinbase) are known to the recipients already. For sufficiently long-running nodes, in practice this is very often the case. As soon as there is at least one transaction missing, a roundtrip is needed which effectively triples the block relay time on each hop (CMPCTBLOCK-GETBLOCKTXN-BLOCKTXN instead of just CMPCTBLOCK). So generally, adding transactions to your own blocks without even (trying to) relay them to the network is a bad idea for the network, and for the miner personally, as they have an incentive to get their blocks propagated as swiftly as possible to other miners. |
I think this still contributes to the functionality. |
It's extremely dangerous because the consensus rules are subtle and no reasonable user expects an exposed option that may cause their blocks to be rejected as invalid. Nothing in my comment said anything about malice by miners, in fact I specifically said "unintentionally". Most causes of non-standardness are intentional reservations for future consensus rules, if miners mine them it makes it much harder to safely update the consensus rules and risks causing their blocks to get orphaned. There are policy elements that can be safely bypassed, e.g. min-fee or dust limit-- so someone intentionally bypassing for those reasons would inadvertently bypass other policy checks and potentially fork the consensus and get themselves orphaned. And again, you cannot validate these rules for yourself externally without essentially a copy of the script interpreter. The fact that someone could edit the software and footgun themselves is not an argument for exposing functionality that has a bad, harmful, and dangerous practice built in. It's an argument against it-- if anything-- because if there is a legitimate reason then there is still a way. |
convinced👍 |
(slightly related to #20753 ) |
It doesn't need to. Maybe we should add logic to include any policy-rejected transactions in the initial compact block? |
Function:
sendrawtransaction
+prioritiserawtransaction
-relay=true
-policy
Rationale: I guess that many pool operators have implemented this in their forks already. Adding it here makes it more convenient for them.
This allows them to mine their high-fee/nonstandard transactions for free.
For example, if a miner needs to consolidate lots of outputs, this allows them to finish this in one large tx and for free.
I might need to rebase it after #21061
Would you
Concept ACK
it?I need to add tests so that even https://github.com/bitcoin/bitcoin/blob/master/test/functional/p2p_segwit.py#L1466-L1468 passes with this feature enabled.